Delete single arch image fix #17
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where deletion operations only worked on multi-arch (manifest/list) images and ignored single-arch images. The fix ensures both image types can be deleted correctly, and also improves HTTP DELETE handling to prevent IncompleteRead errors.
Changes:
- Modified deletion logic to support both manifest/list and single-arch image types
- Added response body consumption for DELETE requests to prevent IncompleteRead errors
- Updated documentation to reflect the corrected deletion behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Docker/Sonar/sonar.py | Fixed DELETE request handling and expanded deletion type check to include both 'manifest/list' and 'image' types |
| Docker/Sonar/README.md | Updated flag descriptions to reflect corrected deletion behavior for both multi-arch and single-arch images |
| CHANGELOG.md | Added entry documenting the deletion fix and improved logging |
Comments suppressed due to low confidence (6)
Docker/Sonar/sonar.py:289
- This comment appears to contain commented-out code.
# if not is_list and not include_all:
# return []
Docker/Sonar/sonar.py:6
- Import of 'csv' is not used.
import csv
Docker/Sonar/sonar.py:14
- Import of 'datetime' is not used.
from datetime import datetime
Docker/Sonar/sonar.py:21
- Import of 'Text' is not used.
from rich.text import Text
Docker/Sonar/sonar.py:607
- 'except' clause does nothing but pass and there is no explanatory comment.
except Exception:
Docker/Sonar/sonar.py:642
- 'except' clause does nothing but pass and there is no explanatory comment.
except Exception:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### Sonar | ||
| - Added `--filter` flag which accepts the (Package Search Syntax)[https://docs.cloudsmith.com/artifact-management/search-filter-sort-packages] string. e.g. `--filter "downloads:>0"`. | ||
| - Deletion now correctly works against non multi-arch images. Previously they where ignored and only multi-arch images were being deleted. Additionally improved log output for deletions. |
There was a problem hiding this comment.
Spelling error: "where" should be "were" in the phrase "Previously they where ignored".
| - Deletion now correctly works against non multi-arch images. Previously they where ignored and only multi-arch images were being deleted. Additionally improved log output for deletions. | |
| - Deletion now correctly works against non multi-arch images. Previously they were ignored and only multi-arch images were being deleted. Additionally improved log output for deletions. |
| | `--delete-all` | Wipes all images and manifest lists detected by the scan. | | ||
| | `--force` | Force deletion without interactive prompt. | | ||
| | `--detailed` | Shows child digests for multi-arch images. | | ||
| | `--untagged` | Show untagged images with no manifest/list (orphaned). | |
There was a problem hiding this comment.
The description for the --untagged flag is unclear. The phrase "Show untagged images with no manifest/list (orphaned)" is ambiguous. Based on the code, this flag shows both untagged manifest lists AND orphaned single images (images with no tags and not referenced by any manifest list). Consider rephrasing to something clearer like "Show untagged manifest lists and orphaned images."
| # Fix: Consume content to prevent IncompleteRead errors even if 0 bytes expected | ||
| try: | ||
| _ = response.read() | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
📄 Summary
🔍 Related Issues
Link to any related GitHub issues (e.g.,
Fixes #12,Closes #34):🧪 Type of Change
Please check the relevant type tag for this PR title:
[FIX]Bug fix[NEW]New thing[REFACTOR]Internal changes such as code restructuring or optimization that does not alter functionality[DOC]Documentation-only changes[CHORE]Maintenance, cleanup, or CI configuration🧪 How Has This Been Tested?
Describe how you tested your changes. Include CI runs, local tests, manual verification, or screenshots if applicable.
📸 Screenshots (if applicable)
If UI or logs are affected, include before/after screenshots or output.
✅ Checklist